[#822] Replace dual-render with CSS grid-template-areas#825
[#822] Replace dual-render with CSS grid-template-areas#825realproject7 merged 2 commits intomainfrom
Conversation
…e-areas Stats grid and CTA button were rendered twice in DOM (hidden sm:block + sm:hidden), causing MarketCapBox and DeadlineCountdown to mount twice with duplicate API calls and timers. Now rendered once and repositioned via grid-template-areas for mobile vs desktop layout. Fixes #822 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
project7-interns
left a comment
project7-interns
left a comment
There was a problem hiding this comment.
LGTM. Grid-template-areas approach correctly eliminates the dual-render pattern. Stats and CTA mount once, grid columns preserve the 100px/160px cover sizing, and the responsive sm: reset on CTA width (w-auto) matches the previous default. No visual or logic regressions.
project7-interns
left a comment
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Summary
The single-render grid approach is the right fix for issue #822, but this version introduces a layout regression when priceInfo is missing.
Findings
- [medium] The new
statsgrid item is always rendered withmt-4 sm:mt-6, even when bothstatsGridandAddPlotButtonrender nothing. On untokenized stories, that leaves an empty spacer below the header for all viewers, and when only the CTA is present it adds extra top spacing versus the old layout.- File:
src/app/story/[storylineId]/page.tsx:384 - Suggestion: Keep the grid-area wrapper marginless and apply the responsive top margin only to
statsGrid, so the CTA-only / empty states preserve the previous spacing.
- File:
Decision
Requesting changes because the issue requires no visual regressions, and this change alters the header spacing for stories without price data.
Fixes empty vertical space on untokenized stories where priceInfo is null and AddPlotButton renders null. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: APPROVE
Summary
The follow-up commit fixes the header spacing regression from the earlier review while preserving the single-render grid-template-areas solution for issue #822.
Findings
- None. The
mt-4 sm:mt-6spacing now applies only whenstatsGridexists, so untokenized stories no longer get empty spacer height and CTA-only states keep their previous spacing.
Decision
Approving because the PR now meets the acceptance criteria: stats and CTA render once in the DOM, side effects mount once, and the prior visual regression is resolved.
2 participants
Summary
hidden sm:block+sm:hiddendual-render pattern with CSSgrid-template-areas'cover info' / 'stats stats')'cover info' / '. stats')Test plan
Fixes #822
🤖 Generated with Claude Code